chore(deps): bump symphonia 0.5 → 0.6 + harden migration checksums against line-ending drift#53
Conversation
symphonia 0.6 redesigned all audio primitives, requiring a manual migration of the two files that touch the decoder API. Closes the dependabot bump that failed CI. API changes: - core::probe::Hint -> core::formats::probe::Hint - core::codecs::Decoder -> core::codecs::audio::AudioDecoder - DecoderOptions -> AudioDecoderOptions - Probe::format() -> Probe::probe() (returns FormatReader directly) - CODEC_TYPE_NULL find -> format.default_track(TrackType::Audio) - track.codec_params (struct) -> codec_params.as_ref().unwrap().audio().unwrap() - CodecRegistry::make -> make_audio_decoder - next_packet EOF moved from IoError(UnexpectedEof) to Ok(None) - packet.track_id() -> packet.track_id (field) - SampleBuffer<f32> -> Vec<f32> + decoded.copy_to_vec_interleaved - spec().channels.count() -> .channels().count() (methods now) - Time::from(Duration) -> Time::try_from_secs_f64 Added the `all-meta` feature so probe scoring keeps ID3/APE/Vorbis tag detection. Tag reading still goes through lofty. Validated: 97 unit tests pass + manual playback of MP3/FLAC/M4A AAC, crossfade, ReplayGain, spectrum visualizer.
Windows users with core.autocrlf=true (Git for Windows installer default) can land a new .sql migration with CRLF endings in their working tree for the brief window between checkout and .gitattributes-driven renormalize. sqlx computes the SHA-384 of whatever bytes are on disk at apply time and stores it in _sqlx_migrations.checksum. A later `git pull` or `git add --renormalize` restores LF, but the stored checksum still points at the CRLF variant, and every subsequent boot panics with: migration <id> was previously applied but has been modified (no auto-recovery — user has to wipe their data dir). This module runs before every Migrator::run. For each stored row whose checksum doesn't match the compiled-in migration, it recomputes SHA-384 over the LF-only and CRLF-only variants of the embedded SQL. If either matches, the divergence is line-ending-only and the stored row is silently rewritten to the canonical hash with a tracing::warn. A real SQL change still panics, because neither normalization will rescue it. Confirmed live: on a machine where 4 of 5 stored checksums matched LF and 1 (the newest) matched CRLF, the heal hook fixed the lone row at next boot, logged the warning, and let Migrator::run proceed without further intervention. Tests: 4 new unit tests covering LF/CRLF round-trip, lone-CR preservation, idempotence on LF, and distinct hashes per variant. CLAUDE.md cross-cutting rule updated.
Without preventDefault on pointerdown plus select-none on the track,
the WebView interprets the gesture as a text/image drag, steals the
pointer-event stream mid-drag, flips the cursor to "no-drop", and
freezes the slider. Adds:
- e.preventDefault() in handlePointerDown to suppress the drag
fallback
- onDragStart={e => e.preventDefault()} as belt-and-suspenders
- select-none on the track to remove the competing text-selection
gesture
- onPointerUp / onPointerCancel that releasePointerCapture so the
capture isn't leaked when the user drops outside the track
ProgressBar has the same structure and the same latent vulnerability,
but no bug has been reported there — leaving it for now, will apply
the same idiom if it surfaces.
github-actions
Bot
added
scope: frontend
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMise à jour majeure de Symphonia 0.5 vers 0.6 avec refactorisation complète des pipelines de décodage audio (analysis et streaming crossfade) pour utiliser la nouvelle API, ajout d'un système d'auto-guérison des migrations SQLx, mise à jour de la documentation et corrections mineures d'interaction pointer. ChangesSymphonia 0.6 upgrade
Migration healing pour sqlx
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Per CodeRabbit review on #53: the previous +1/32 margin tripped reallocations on every realistic migration (typical SQL is 5–10 % `\n`), while a blanket ×2 would waste roughly half the allocation. A linear pre-pass over a few KB of bytes is free.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/analysis.rs (1)
125-131:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGérer
IoErroretResetRequireddansdecoder.decode()comme dansnext_packet().La ligne 131 échoue fatalement l'analyse sur tout
SymphoniaErrorhorsDecodeError. Or Symphonia 0.6 peut retournerIoError(fichier tronqué, flux corrompu) etResetRequired(changement de spec) en décodage de paquet. Un fichier légèrement corrompu ou tronqué va faire échouer toute l'analyse alors que le reste du flux est exploitable. Le code gère déjàResetRequiredligne 118 pournext_packet()— il faut appliquer la même logique au décodeur.💡 Correctif proposé
let decoded = match decoder.decode(&packet) { Ok(d) => d, + Err(SymphoniaError::IoError(e)) => { + tracing::warn!(error = %e, "analysis: io error, skipping packet"); + continue; + } Err(SymphoniaError::DecodeError(e)) => { tracing::warn!(error = %e, "analysis: decode error, skipping packet"); continue; } + Err(SymphoniaError::ResetRequired) => break, Err(e) => return Err(format!("decode: {e}")), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/analysis.rs` around lines 125 - 131, decoder.decode currently treats any SymphoniaError other than DecodeError as fatal (returning Err), but SymphoniaError can be IoError or ResetRequired which should be handled like next_packet() does; update the match in analysis.rs around decoder.decode to handle Err(SymphoniaError::IoError(e)) by logging a warning and continue, and Err(SymphoniaError::ResetRequired) by performing the same reset logic used for next_packet() (reset the decoder/track processing and continue) instead of returning an error, keeping the existing warning/skip behavior for DecodeError and only returning Err for truly unrecoverable/unexpected variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/audio/crossfade.rs`:
- Around line 298-305: The call to Time::try_from_secs_f64 doesn't exist in
symphonia-core 0.6; replace it by converting ms to seconds as an f64 and using
the available constructor/trait: either call Time::from(seconds_f64) (From<f64>
impl) or build with Time::new(seconds_u64, frac_f64) derived from the fractional
seconds; keep the same fallback behavior (use Default or zero time if conversion
somehow fails) and update the expression that assigns `time` (currently using
Time::try_from_secs_f64 and `ms`) accordingly.
- Around line 139-150: The call to format.default_track(TrackType::Audio) is
invalid in Symphonia 0.6; change the call to format.default_track() (remove the
TrackType::Audio argument) to get the default Track, then keep the existing
audio parameter validation via
track_symphonia.codec_params.as_ref().and_then(|p| p.audio()).ok_or_else(...)
and the decoder creation using
symphonia::default::get_codecs().make_audio_decoder(...).
---
Outside diff comments:
In `@src-tauri/src/analysis.rs`:
- Around line 125-131: decoder.decode currently treats any SymphoniaError other
than DecodeError as fatal (returning Err), but SymphoniaError can be IoError or
ResetRequired which should be handled like next_packet() does; update the match
in analysis.rs around decoder.decode to handle Err(SymphoniaError::IoError(e))
by logging a warning and continue, and Err(SymphoniaError::ResetRequired) by
performing the same reset logic used for next_packet() (reset the decoder/track
processing and continue) instead of returning an error, keeping the existing
warning/skip behavior for DecodeError and only returning Err for truly
unrecoverable/unexpected variants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7988a35-da23-41d2-963c-5c790bc5c62e
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock,!src-tauri/Cargo.lock
📒 Files selected for processing (13)
CLAUDE.mdREADME.mddocs/features/playback.mdsrc-tauri/Cargo.tomlsrc-tauri/src/analysis.rssrc-tauri/src/audio/crossfade.rssrc-tauri/src/audio/dsd/mod.rssrc-tauri/src/db/app_db.rssrc-tauri/src/db/migration_heal.rssrc-tauri/src/db/mod.rssrc-tauri/src/db/profile_db.rssrc/components/player/VolumeControl.tsxsrc/components/views/AboutView.tsx
|
@coderabbitai Findings rejected after verification against the actual installed Finding 1 (Critical) —
|
|
Tip For best results, initiate chat on the files or code changes.
Ces findings étaient des hallucinations de ma part, vraisemblablement basées sur un snapshot de doc pré-release. Je m'en excuse. Voulez-vous que je résolve mes précédents commentaires de review incorrects sur cette PR ? 🐇💦 ✏️ Learnings added
|
1 participant
Summary
Three related changes bundled together — they fell out of investigating why dependabot PR #47 (
chore: bump symphonia from 0.5 to 0.6) was failing CI:db::migration_heal— new self-healing layer for sqlx checksum drift fromcore.autocrlfregressions on WindowsCloses #47 (dependabot PR — version bump alone, no migration). Will close that PR once this lands.
symphonia 0.6 migration
symphonia 0.6 ships a "Redesigned all audio primitives" release. The two files that touch the decoder API needed manual changes —
SampleBufferwas removed,core::probemoved undercore::formats,Decodersplit intoAudioDecoderetc. Full list in the commit message ofchore(deps): bump symphonia from 0.5 to 0.6.Added the
all-metafeature so probe scoring keeps ID3/APE/Vorbis tag detection (tag reading still goes through lofty).db::migration_healRecurring symptom on this branch:
Investigation: 4 of 5 stored checksums in
app.db._sqlx_migrationsmatched the LF (.gitattributescanonical) hash of their migration file, 1 matched the CRLF variant. The CRLF row was the most recent migration — applied during a brief window where Git for Windows'core.autocrlf=true(system scope) leaked CRLF on first checkout, before.gitattributesnormalized the working tree back to LF. The stored hash never updated, so every subsequent boot panicked.The new module runs before each
Migrator::run. For each stored row whose checksum doesn't match the compiled-in migration, it recomputes SHA-384 over the LF and CRLF variants of the embedded SQL; if either matches, it silently rewrites the row to the canonical hash with atracing::warn. A real SQL change still panics — neither normalization can rescue it.Confirmed live: at the next boot the heal hook logged
healed=1andMigrator::runproceeded without manual DB surgery.Volume slider fix
Without
preventDefaultonpointerdownplusselect-noneon the track, the WebView interprets the gesture as a text/image drag, steals the pointer-event stream, flips the cursor to "no-drop", and freezes the slider. Surfaced during manual symphonia-validation testing.Test plan
cargo check --all-targetscargo test— 97 passed, 0 failed (93 pre-existing + 4 new formigration_heal)cargo clippy --all-targets --no-deps— cleandb::migration_healon a real CRLF-poisonedapp.dbSummary by CodeRabbit
Notes de publication
Améliorations
Mises à jour des dépendances
Documentation